Skip to content

SNOW-2912540: decouple LocalTestOOBTelemetryService from connector.telemetry_oob - #4309

Open
sfc-gh-fpawlowski wants to merge 8 commits into
SNOW-2912540-inline-pandas-staging-helpersfrom
SNOW-2912540-decouple-oob-telemetry
Open

SNOW-2912540: decouple LocalTestOOBTelemetryService from connector.telemetry_oob#4309
sfc-gh-fpawlowski wants to merge 8 commits into
SNOW-2912540-inline-pandas-staging-helpersfrom
SNOW-2912540-decouple-oob-telemetry

Conversation

@sfc-gh-fpawlowski

@sfc-gh-fpawlowski sfc-gh-fpawlowski commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

The UD's telemetry_oob.TelemetryService is a no-op stub (BD#45) that is missing batch_size, which LocalTestOOBTelemetryService.add() uses — causing an AttributeError at runtime.

Makes LocalTestOOBTelemetryService fully standalone by:

  • Removing from snowflake.connector.telemetry_oob import TelemetryService and the inheritance
  • Adding its own double-checked-lock singleton (get_instance())
  • Initialising self.queue, self.batch_size, and self._enabled directly in __init__
  • Adding close() (delegates to flush(), called by atexit)
  • Fixing the pre-existing self._enable typo (should have been _enabled)

SecretDetector (also from the connector) is now decoupled too — moved into mock/_secret_detector.py as a literal copy of the Universal Driver's _common/secret_detector.py (the shim added in drivers#598 purely to satisfy this one Snowpark import; SecretDetector was never a real UD capability). Only the backward-compatibility-specific mechanism was removed (@backward_compatibility, install_backward_compatibility_getattr), since that has no equivalent in Snowpark — everything else (the class, MaskedMessageData, the 3-tuple return, logging.Formatter integration, masked_text = str(ex) on failure) is unchanged from the UD source, including a known real finding already flagged by the security bot on drivers#598 that isn't fixed as part of this move.

Note that OOB telemetry is disabled on the backend anyway, so this masking is defense-in-depth on the outbound payload, not a response to active exploitation — but export_queue_to_string() still feeds a real requests.Session().post() to a live analytics endpoint, so it's not a safe no-op either. If full secret-masking becomes a real requirement for the Universal Driver itself later, the right fix is a DependencyRequest to implement it once in sf_core, not to re-add a per-language regex port.

After this, mock/_telemetry.py has zero remaining imports from snowflake.connector.

Checklist

  • I acknowledge that I have ensured my changes to be thread-safe

Stack (via Graphite)

🤖 Generated with Claude Code

sfc-gh-fpawlowski commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

@codecov-commenter

codecov-commenter commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (b7cbf51) to head (f6e0c0d).

Additional details and impacted files
@@                             Coverage Diff                             @@
##           SNOW-2912540-inline-pandas-staging-helpers    #4309   +/-   ##
===========================================================================
  Coverage                                       95.24%   95.24%           
===========================================================================
  Files                                             171      171           
  Lines                                           44794    44794           
  Branches                                         7687     7687           
===========================================================================
  Hits                                            42664    42664           
  Misses                                           1343     1343           
  Partials                                          787      787           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from SNOW-2912540-inline-pandas-staging-helpers to graphite-base/4309 August 20, 2026 16:27
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-decouple-oob-telemetry branch from c202a6b to 069c7ff Compare August 20, 2026 16:28
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from graphite-base/4309 to SNOW-2912540-inline-pandas-staging-helpers August 20, 2026 16:28
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from SNOW-2912540-inline-pandas-staging-helpers to graphite-base/4309 August 21, 2026 08:24
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-decouple-oob-telemetry branch from 4d5e471 to c921b91 Compare August 21, 2026 08:30
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from graphite-base/4309 to SNOW-2912540-inline-pandas-staging-helpers August 21, 2026 08:30
sfc-gh-fpawlowski and others added 4 commits September 2, 2026 06:43
…lemetry_oob

The UD's telemetry_oob.TelemetryService is a no-op stub (BD#45) that lacks
batch_size, causing an AttributeError at runtime. Make LocalTestOOBTelemetryService
standalone: add its own singleton, queue, batch_size, _enabled, and close().
Also fixes the pre-existing _enable typo (should have been _enabled).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The decoupling from connector.telemetry_oob ported add()/flush()/enable()/
disable() but missed size() (connector/telemetry_oob.py:542,
`return self.queue.qsize()`), which several tests
(tests/mock/test_oob_telemetry.py, tests/mock/test_multithreading.py) call
directly on the service instance.
mock/_telemetry.py imported snowflake.connector.secret_detector.SecretDetector
unconditionally -- the only reason UD PR #598 ports SecretDetector into the
Universal Driver at all is to satisfy this one Snowpark import (SecretDetector
is not otherwise used or maintained by UD). Per BehaviorDifferences.yaml #45,
OOB telemetry is disabled on the backend anyway, so this masking is
defense-in-depth on the outbound payload, not a response to active
exploitation -- but export_queue_to_string() feeds a real
requests.Session().post() to a live analytics endpoint, so it's not a safe
no-op either.

Ports only what _telemetry.py actually calls (mask_secrets -> masked string),
from UD's actual _common/secret_detector.py source (PR #598) rather than the
older, less complete real v4 connector's version -- verified UD's copy has
3 additional maskers (OAuth tokens, OAuth client secrets, passcodes) plus a
PASSWORD_PATTERN false-positive fix and a wire-format fix to
CONNECTION_TOKEN_PATTERN, all needed for correctness. Drops everything that
exists solely for UD's backward-compat contract: @backward_compatibility,
MaskedMessageData's 3-tuple return (only masked_text is ever read),
logging.Formatter inheritance, and the classmethod-vs-staticmethod workaround.

Fixes the exception-handling branch rather than porting it straight: the
legacy/UD code does masked_text = str(ex) on failure, which can leak
exception-reflected input. Returns a static sentinel instead -- this was
already flagged as a real finding by the security bot on UD #598.

After this, mock/_telemetry.py has zero remaining imports from
snowflake.connector.

Test coverage: ported every behavioral test from UD PR #598's
test_secret_detector.py TestMaskSecrets class (27 cases via pytest count),
adapted from the 3-tuple/class-method API to the plain-function API here,
plus one exception-handling test adapted to assert the static sentinel
instead of the leaked exception text. Dropped TestFormatter and the
logging.Formatter-specific exception tests -- not applicable, since
mask_secrets is a plain function here, not a logging.Formatter subclass.
Corrects the previous commit: this is now an exact copy of UD's
_common/secret_detector.py (drivers#598), not a collapsed rewrite. Keeps the
SecretDetector class, MaskedMessageData, the (is_masked, masked_text, err_str)
3-tuple return, classmethods, logging.Formatter inheritance, format(),
create_formatting_error_log(), and masked_text = str(ex) on failure --
unchanged, even though the str(ex) behavior is a known real finding (flagged
by the security bot on UD #598); fixing it is a separate call for whoever
reviews this, not bundled into a move.

Only removed what's physically impossible to keep: @backward_compatibility,
install_backward_compatibility_getattr and its import, and the trailing
install_backward_compatibility_getattr(__name__) call -- none of that
mechanism exists in snowpark-python. The mask_secrets() docstring explaining
why the maskers are classmethods (to survive @backward_compatibility
stashing the class out of module globals) is left as-is even though that
rationale no longer applies here -- not silently deleted or rewritten as
part of this move.

_telemetry.py's call site goes back to unpacking the 3-tuple unmodified;
test_secret_detector.py is now a full copy of UD's test suite (TestMaskSecrets,
TestMaskSecretsExceptionHandling, TestFormatter), not just the masking-behavior
subset -- format()/logging.Formatter are kept, so their tests are too.
@cursor
cursor Bot force-pushed the SNOW-2912540-inline-pandas-staging-helpers branch from 4292ad1 to 5812065 Compare September 2, 2026 06:45
@cursor
cursor Bot force-pushed the SNOW-2912540-decouple-oob-telemetry branch from 3714893 to f53b5c4 Compare September 2, 2026 06:45
Comment thread src/snowflake/snowpark/mock/_secret_detector.py
cursoragent and others added 4 commits September 2, 2026 17:51
pre-commit insert-license failed Check linting from this PR through #4313.

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…912540-decouple-oob-telemetry

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
…912540-decouple-oob-telemetry

Co-authored-by: Filip Pawłowski <sfc-gh-fpawlowski@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants